Skip to content

Conversation

@sanderegg
Copy link
Member

@sanderegg sanderegg commented Jun 6, 2025

What do these changes do?

only require sticky connection for

/v0/projects?fromStudy=,	  
/v0/projects:clone, 
/v0/projects/{project_Id}/nodes/{node_id}:stop, /v0/storage/locations/{location_id}/paths/{path}:size, /v0/storage/locations/{location_id}/-/paths:batchDelete, 
/v0/storage/locations/{location_id}/export-data
/v0/tasks-legacy/

These entrypoints rely on in-process tasks to be available, otherwise these actions fail.
Once the tasks get completely process agnostic, the sticky connection shall be completely removed and then scaling up webservers shall have a direct effect when the number of users increases (see #7778).

Also this PR sets the number of replicas of the webserver to 2 to decrease the chances of adding in-process tasks again.

NOTE: This is a rather risky PR on which we agreed to merge only after the current release

Related issue/s

How to test

Dev-ops

@sanderegg sanderegg added this to the Bazinga! milestone Jun 6, 2025
@sanderegg sanderegg self-assigned this Jun 6, 2025
@sanderegg sanderegg added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Jun 6, 2025
@codecov
Copy link

codecov bot commented Jun 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.93%. Comparing base (73a45bc) to head (72c3fb2).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7839      +/-   ##
==========================================
+ Coverage   87.89%   87.93%   +0.04%     
==========================================
  Files        1837     1830       -7     
  Lines       71032    70842     -190     
  Branches     1219     1219              
==========================================
- Hits        62434    62296     -138     
+ Misses       8255     8203      -52     
  Partials      343      343              
Flag Coverage Δ
integrationtests 64.22% <ø> (+0.02%) ⬆️
unittests 86.52% <ø> (+0.03%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library 93.92% <ø> (ø)
pkg_dask_task_models_library 79.62% <ø> (ø)
pkg_models_library 93.19% <ø> (ø)
pkg_notifications_library 85.26% <ø> (ø)
pkg_postgres_database 88.16% <ø> (ø)
pkg_service_integration 69.92% <ø> (ø)
pkg_service_library 71.81% <ø> (ø)
pkg_settings_library 90.90% <ø> (ø)
pkg_simcore_sdk 85.05% <ø> (+0.05%) ⬆️
agent 96.29% <ø> (ø)
api_server 91.76% <ø> (ø)
autoscaling 96.03% <ø> (ø)
catalog 92.29% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 91.57% <ø> (-0.23%) ⬇️
datcore_adapter 97.94% <ø> (ø)
director 76.73% <ø> (ø)
director_v2 91.13% <ø> (+0.07%) ⬆️
dynamic_scheduler 96.69% <ø> (ø)
dynamic_sidecar 90.09% <ø> (ø)
efs_guardian 89.65% <ø> (ø)
invitations 93.00% <ø> (ø)
payments 92.57% <ø> (ø)
resource_usage_tracker 89.09% <ø> (ø)
storage 87.78% <ø> (+0.24%) ⬆️
webclient ∅ <ø> (∅)
webserver 87.59% <ø> (-0.02%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73a45bc...72c3fb2. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sanderegg sanderegg force-pushed the only-required-sticky-endpoints branch 2 times, most recently from 398cf31 to a14651e Compare June 10, 2025 15:48
@sanderegg sanderegg changed the title 🎨Only require sticky connection on specific endpoints 🎨Only require sticky connection on specific endpoints (🚨🚨) Jun 10, 2025
@sanderegg sanderegg marked this pull request as ready for review June 10, 2025 15:57
Copy link
Member Author

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocked until release v1.83.0 is out
release was postponed so this can go in now.

@sanderegg sanderegg added the 🤖-do-not-merge (optional) blocks Mergify from merging the PR label Jun 10, 2025
@sanderegg sanderegg requested review from Copilot and odeimaiz June 10, 2025 16:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR scopes sticky session routing to only the required endpoints and increases the webserver replica count to improve resilience and allow early detection of in-process tasks.

  • Scale webserver service to 2 replicas for early detection of in-process tasks
  • Remove global sticky session labels and introduce a dedicated Traefik service/router for specific endpoints
  • Clean up blank lines in compose files and add new Python extra paths in VSCode settings

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
services/docker-compose.yml Added replicas: 2, removed global sticky labels, introduced dedicated sticky service
services/docker-compose.local.yml Updated local sticky service label and secure flag
.vscode/settings.template.json Added common-library and dask-task-models-library to Python extraPaths
Comments suppressed due to low confidence (2)

services/docker-compose.local.yml:162

  • [nitpick] Consider adding sticky.cookie.httponly and sticky.cookie.samesite labels in local compose, so dev mirrors production cookie security settings.
-        - traefik.http.services.${SWARM_STACK_NAME}_webserver_sticky.loadbalancer.sticky.cookie.secure=false

services/docker-compose.yml:1359

  • [nitpick] This single-line array is harder to read and maintain compared to the previous multiline format; consider restoring multiline YAML for clarity.
[ "redis-server", "--save", "60 1", "--loglevel", "verbose", "--databases", "10", "--appendonly", "yes", "--requirepass", "${REDIS_PASSWORD}" ]

Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very very nice effort, thx

@sanderegg sanderegg force-pushed the only-required-sticky-endpoints branch from 2b9a1f2 to 0ce2954 Compare June 11, 2025 06:02
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx

Copy link
Contributor

@YuryHrytsuk YuryHrytsuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, let me know what is the exact requirement for static-webserver replica's count.

I will then create a corresponding PR for osparc-ops-environments repo

@sanderegg sanderegg force-pushed the only-required-sticky-endpoints branch from 0ce2954 to 3406ee6 Compare June 11, 2025 20:54
@sanderegg sanderegg added 🤖-automerge marks PR as ready to be merged for Mergify and removed 🤖-do-not-merge (optional) blocks Mergify from merging the PR labels Jun 11, 2025
@sanderegg
Copy link
Member Author

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Jun 11, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at cf51daf

@sanderegg sanderegg force-pushed the only-required-sticky-endpoints branch from 3406ee6 to 72c3fb2 Compare June 12, 2025 05:39
@sonarqubecloud
Copy link

@sanderegg sanderegg requested a review from YuryHrytsuk June 12, 2025 05:47
Copy link
Contributor

@YuryHrytsuk YuryHrytsuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mergify mergify bot merged commit cf51daf into ITISFoundation:master Jun 12, 2025
143 of 147 checks passed
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only use sticky connections for specific endpoints where it is required

7 participants